Launch/Open/GoTo: Code by Jay-o-Way · Pull Request #32516 · microsoft/PowerToys

Jay-o-Way · 2024-04-18T12:24:38Z

Summary of the Pull Request

Split from #32351
Closely linked to #32514 and #32519.

PR Checklist

Closes: "Open", "Launch", "Go to"... #30685
Communication: I've discussed this with core contributors already. If work hasn't been agreed, this work might be rejected
Localization: All end user facing strings can be localized
Dev docs: Added/updated

Detailed Description of the Pull Request / Additional comments

Validation Steps Performed

Build

jaimecbernardo

Thanks for opening the PR.
I see the PR will contain changes that seem to be changing localization ids, retriggering the localization for all these strings.
It also changes names of telemetry events, which means it'll cause confusion in the names of events received and they'll be treated like two different events across versions.
I paused my review, since the PR seems to have many of these. These are not visible for the user, so can we please not make these changes as they'll cause more work / confusion for changes that won't affect the user in any way?

jaimecbernardo · 2024-05-29T12:39:58Z

src/modules/Hosts/Hosts/HostsXAML/App.xaml.cs

-        /// Invoked when the application is launched.
-        /// </summary>
-        /// <param name="args">Details about the launch request and process.</param>
        protected override void OnLaunched(LaunchActivatedEventArgs args)


Why are we deleting these comments? 🤔

Uhm, I have forgotten now. But I would guess that this comment is so obvious?

Please let's keep the changes at the scope of the PR?

Reverted ✔️

jaimecbernardo · 2024-05-29T12:41:54Z

src/modules/fancyzones/FancyZonesLib/Resources.resx

  <data name="Setting_Description_RestoreSize" xml:space="preserve">
    <value>Restore the original size of windows when unsnapping</value>
  </data>
-  <data name="Setting_Launch_Editor_Label" xml:space="preserve">


These are internal labels which will retrigger localization. I don't think it's worth retriggering localization for all these strings just to change internal names.

Hm, I see the point. Reverted ✔️

LOL this entire file seems to be obsolete
@SeraphimaZykova can you confirm?

Yes, please revert the labels. Revert the changes from xaml / resource getting in code in case those are changed as well.

Already done after your comment, check out the latest commits :)

jaimecbernardo · 2024-05-29T12:42:39Z

src/modules/fancyzones/FancyZonesLib/trace.cpp

 #define EventKeyDownKey "FancyZones_OnKeyDown"
 #define EventZoneSettingsChangedKey "FancyZones_ZoneSettingsChanged"
-#define EventEditorLaunchKey "FancyZones_EditorLaunch"
+#define EventEditorOpenKey "FancyZones_EditorOpen"


This will make old events not match new ones. It doesn't seem to make sense.

I see. I'm not familiar with telemetry. Reverted ✔️

So: revert any/all trace.cpp and trace.h files, right?

Yes, please :) And the calls to those as well.

Already done after your comment, check out the latest commits :)

Jay-o-Way · 2024-05-29T15:14:13Z

src/settings-ui/Settings.UI/SettingsXAML/OOBE/Views/OobeRegistryPreview.xaml.cs

        private void Launch_RegistryPreview_Click(object sender, Microsoft.UI.Xaml.RoutedEventArgs e)
        {
-            ShellPage.SendDefaultIPCMessage("{\"action\":{\"RegistryPreview\":{\"action_name\":\"Launch\", \"value\":\"\"}}}");
+            ShellPage.SendDefaultIPCMessage("{\"action\":{\"RegistryPreview\":{\"action_name\":\"Open\", \"value\":\"\"}}}");


@jaimecbernardo I wonder if this will be a "breaking change" too?

You're right, it might be 🤔 Let's leave it as launch, I guess?

Already done after your comment, check out the latest commits :)

src/modules/fancyzones/FancyZonesLib/Settings.cpp

Jay-o-Way · 2024-05-29T15:18:52Z

src/modules/fancyzones/FancyZonesLib/Resources.resx

  <data name="Setting_Description_RestoreSize" xml:space="preserve">
    <value>Restore the original size of windows when unsnapping</value>
  </data>
-  <data name="Setting_Launch_Editor_Label" xml:space="preserve">


Hm, I see the point. Reverted ✔️

src/modules/fancyzones/FancyZonesLib/SettingsConstants.h

Jay-o-Way · 2024-05-29T15:21:07Z

src/modules/fancyzones/FancyZonesLib/trace.cpp

 #define EventKeyDownKey "FancyZones_OnKeyDown"
 #define EventZoneSettingsChangedKey "FancyZones_ZoneSettingsChanged"
-#define EventEditorLaunchKey "FancyZones_EditorLaunch"
+#define EventEditorOpenKey "FancyZones_EditorOpen"


I see. I'm not familiar with telemetry. Reverted ✔️

Jay-o-Way · 2024-08-01T11:51:10Z

helloooo???

This was referenced Apr 18, 2024

Launch/Open/GoTo: Comments #32514

Closed

Launch/Open/GoTo: Userfacing strings #32519

Closed

Jay-o-Way marked this pull request as ready for review April 18, 2024 13:17

Jay-o-Way requested a review from jaimecbernardo April 18, 2024 13:17

Jay-o-Way added this to the PowerToys 0.81 milestone Apr 18, 2024

Jay-o-Way changed the title ~~Launch open go: Code~~ Launch/Open/GoTo: Code Apr 22, 2024

jaimecbernardo added the revisit-0.82.0 label May 10, 2024

jaimecbernardo suggested changes May 29, 2024

View reviewed changes

Jay-o-Way commented May 29, 2024

View reviewed changes

src/modules/fancyzones/FancyZonesLib/Settings.cpp Outdated Show resolved Hide resolved

Jay-o-Way commented May 29, 2024

View reviewed changes

Jay-o-Way removed this from the PowerToys 0.81 milestone May 29, 2024

Jay-o-Way requested a review from jaimecbernardo May 31, 2024 13:11

Jay-o-Way added 7 commits May 31, 2024 17:45

launch - open

ace2a7a

launched - opened

f0da7ad

launch - go to (settings)

9a7347a

tray menu IDs

e57b1fa

undo OnLaunched - OnOpened

dd48784

quit - close

b958af3

Revert

c166862

Jay-o-Way requested review from jaimecbernardo and removed request for jaimecbernardo July 4, 2024 23:17

Jay-o-Way requested review from crutkas and ethanfangg August 1, 2024 11:49

Jay-o-Way closed this by deleting the head repository Oct 30, 2024

Conversation

Jay-o-Way commented Apr 18, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary of the Pull Request

PR Checklist

Detailed Description of the Pull Request / Additional comments

Validation Steps Performed

Uh oh!

jaimecbernardo left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Jay-o-Way May 29, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Jay-o-Way May 29, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Jay-o-Way May 29, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Jay-o-Way May 29, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Jay-o-Way May 29, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Jay-o-Way commented Aug 1, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Jay-o-Way commented Apr 18, 2024 •

edited

Loading

Jay-o-Way May 29, 2024 •

edited

Loading

Jay-o-Way May 29, 2024 •

edited

Loading

Jay-o-Way May 29, 2024 •

edited

Loading

Jay-o-Way May 29, 2024 •

edited

Loading

Jay-o-Way May 29, 2024 •

edited

Loading